Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: WIP rbac #20864

Draft
wants to merge 314 commits into
base: master
Choose a base branch
from
Draft

feat: WIP rbac #20864

wants to merge 314 commits into from

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Mar 12, 2024

Problem

Very WIP - testing out ideas and feasability
Attempt to implement https://github.com/PostHog/product-internal/pull/577

Changes

  • Major refactor of how we do permissioning at an object and resource level.
  • Standardises ExplicitTeamPermissions, FeatureFlag access, DashboardCollaborators all into a single AccessControl concept.
  • Allows configuring of AccessControls at an object level (i.e. No one can access my Notebook by default unless I give them access)
  • Allows configuring of AccessControls at a resource level (i.e. Generally is someone able to edit Feature flags or not)
  • AccessControls can be specified per-user or per-role

Access controls for a project
Screenshot 2024-09-20 at 11 41 06
Access controls for a resource

Screenshot 2024-09-20 at 11 41 42

TODO

  • Base UI
  • Replace ExplictTeamMembership UI with new one
    • Make default be "all access" with ability to change it to no access.
    • Change routes to always be actions of other endpoints (for editing)
    • Add indicator of whether you can edit access or not
  • Replace FeatureFlag Permissions UI with new one
  • Replace Roles UI with new one
    • Make the default be "view"
    • Fix it so you can deselect an override
  • Put back in paygate controls
  • Feature flag it
  • How to add an access control as you create? If we don't default to scoped to the user then what do we do 🤔
  • Implement new permissions for Projects
  • Implement new permissions for main things for now - Dashboards, Insights, Flags, Notebooks, Experiments, Surveys, Plugins, EAF
    • Fix clashing permissions issues...
    • Fix nested things like for dashboards -> insights
  • Account for the creator of a thing should always be able to see the thing (i guess?)
  • Optimize caching of various DB calls
  • Fix listing of resources to include access control info efficiently
  • Admins can modify things like a notebook even if it is locked to view only. We likely want to prevent this either at the UI level or something else... Like perhaps being a project admin shouldn't override and instead force them to change the access level for the thing
  • Indicate if a creator exists and that they will always have access to the item

Follow up / out of scope

#21222

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Copy link
Contributor

github-actions bot commented Mar 12, 2024

Size Change: +285 B (0%)

Total Size: 1.05 MB

ℹ️ View Unchanged
Filename Size Change
frontend/dist/toolbar.js 1.05 MB +285 B (0%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

benjackwhite and others added 9 commits May 6, 2024 09:07
# Conflicts:
#	frontend/__snapshots__/scenes-other-settings--settings-organization--dark.png
#	frontend/src/scenes/settings/SettingsMap.tsx
#	posthog/api/dashboards/dashboard.py
#	posthog/api/dashboards/dashboard_templates.py
#	posthog/api/insight.py
#	posthog/api/routing.py
#	posthog/api/test/__snapshots__/test_action.ambr
#	posthog/api/test/__snapshots__/test_annotation.ambr
#	posthog/api/test/__snapshots__/test_feature_flag.ambr
#	posthog/api/test/dashboards/__snapshots__/test_dashboard.ambr
#	posthog/api/test/notebooks/__snapshots__/test_notebook.ambr
#	posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr
#	posthog/test/test_middleware.py
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

benjackwhite and others added 4 commits September 20, 2024 10:31
# Conflicts:
#	.vscode/launch.json
#	ee/api/test/test_action.py
#	ee/api/test/test_team.py
#	ee/models/rbac/role.py
#	ee/urls.py
#	frontend/__snapshots__/scenes-other-settings--settings-organization--dark.png
#	frontend/__snapshots__/scenes-other-settings--settings-organization--light.png
#	frontend/src/layout/navigation-3000/sidepanel/sidePanelLogic.tsx
#	frontend/src/models/dashboardsModel.tsx
#	frontend/src/scenes/notebooks/Notebook/migrations/migrate.test.ts
#	frontend/src/scenes/settings/SettingsMap.tsx
#	frontend/src/scenes/settings/organization/inviteLogic.ts
#	frontend/src/scenes/teamActivityDescriber.tsx
#	frontend/src/types.ts
#	mypy-baseline.txt
#	posthog/api/personal_api_key.py
#	posthog/api/routing.py
#	posthog/api/search.py
#	posthog/api/team.py
#	posthog/api/test/__snapshots__/test_action.ambr
#	posthog/api/test/__snapshots__/test_annotation.ambr
#	posthog/api/test/__snapshots__/test_api_docs.ambr
#	posthog/api/test/__snapshots__/test_insight.ambr
#	posthog/api/test/dashboards/__snapshots__/test_dashboard.ambr
#	posthog/api/test/dashboards/test_dashboard.py
#	posthog/api/test/notebooks/__snapshots__/test_notebook.ambr
#	posthog/api/test/test_action.py
#	posthog/api/test/test_event.py
#	posthog/api/test/test_organization_feature_flag.py
#	posthog/api/test/test_person.py
#	posthog/api/test/test_plugin.py
#	posthog/api/test/test_survey.py
#	posthog/middleware.py
#	posthog/permissions.py
#	posthog/session_recordings/test/__snapshots__/test_session_recordings.ambr
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

19 snapshot changes in total. 0 added, 19 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

12 snapshot changes in total. 0 added, 12 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting Prevents stale-bot from marking the PR as stale.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants